[sync] fix(v2): keep schema data statements off meta tables T6902 - #3620
[sync] fix(v2): keep schema data statements off meta tables T6902#3620tea-artist wants to merge 1 commit into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d24ae0c57
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const userCount = members.filter((m) => m.principalType === PrincipalType.User).length; | ||
| if (await confirmSeat({ role: role as IRole, count: userCount, action: 'invite' })) { |
There was a problem hiding this comment.
When the organization selection contains only departments, userCount is zero, so useSeatConfirm returns immediately and skips both the cloud billing confirmation and the EE license limit. Department collaborators grant access to their users—the backend includes a user's organization departments when resolving collaborators—so assigning a billable role to a department can consume multiple seats. Resolve the department's newly billable members instead of filtering departments out; the same zero-seat assumption is present in the base invitation and department role-change handlers.
Useful? React with 👍 / 👎.
| onCreate={async (ro) => { | ||
| if (!(await confirmSeat({ role: ro.role, count: ro.emails.length, action: 'invite' }))) { | ||
| return false; |
There was a problem hiding this comment.
On an EE instance, ro.emails.length is not necessarily the number of new seats. For example, inviting a user who is already billable through another base or space creates a new collaborator without increasing the instance-wide billable-user count. If the instance is at its seat limit, this calculation rejects that valid invitation because seats + emails.length exceeds the limit. The confirmation needs a server-derived count of newly billable users; the base email flow has the same issue.
Useful? React with 👍 / 👎.
| onUpdate={async (invitationId, role) => { | ||
| if (await confirmSeat({ role, count: 1, action: 'link' })) { | ||
| updateInviteLink({ |
There was a problem hiding this comment.
Updating an existing invitation link from one billable role to another, such as Creator to Editor, does not add a collaborator or consume a seat—the backend only updates the invitation row. Nevertheless this always checks with count: 1, so an EE instance at its seat limit cannot make that role adjustment, and cloud users receive an inaccurate bill-increase confirmation. Compare the existing link role and apply a positive delta only when the transition actually introduces billable access; the base-link handler repeats this behavior.
Useful? React with 👍 / 👎.
| const { confirm, alert } = useConfirm(); | ||
| const queryClient = useQueryClient(); | ||
|
|
||
| const isPaidSpace = isCloud && level != null && level !== BillingProductLevel.Free; |
There was a problem hiding this comment.
On Cloud, level is undefined while useBillingLevel is still fetching, and it remains undefined if that query fails. This classifies the space as unpaid; because isEE is also false, the callback falls through to true. A paid customer who submits an invitation or role change before the usage query resolves therefore bypasses the required bill-increase confirmation. Await or block on the billing query rather than treating an unknown level as Free.
Useful? React with 👍 / 👎.
| queryFn: () => getInstanceUsage().then((res) => res.data), | ||
| staleTime: 0, | ||
| }) | ||
| .catch(() => undefined); |
There was a problem hiding this comment.
When the instance-usage request fails, this catch converts the error to undefined, leaving seatLimit absent and allowing the callback to return true. During any transient usage-service or network failure, the self-hosted seat-limit dialog and the stated hard stop are therefore bypassed, and the billable mutation is attempted without knowing whether capacity remains. Surface the failure or fail closed instead of silently authorizing the action.
Useful? React with 👍 / 👎.
Coverage Report for CI Build 31371708818Coverage at 56.749% (no base build to compare)Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
tea-artist
changed the title
5d24ae0 to
f0f79a9
Compare
tea-artist
changed the title
f0f79a9 to
e600e6e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e600e6e93d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return; | ||
| } | ||
| if (knownCount > 0) { | ||
| ctx.nextGroupPoints.push({ type: GroupPointType.Row, count: knownCount }); |
There was a problem hiding this comment.
When a multi-level group's parent is collapsed, its descendant headers are removed from groupPoints; re-expanding that parent here restores only one aggregate row block and cannot reconstruct those headers. The locally patched state therefore flattens the parent, and useGridAsyncRecords subsequently caches that malformed structure, so on a slow or failed fresh delivery the child groups remain missing and cannot be expanded or collapsed. Preserve the cached subtree for parent expansion, or avoid applying the in-place expansion when only an aggregate count is available.
Useful? React with 👍 / 👎.
| // deliveries re-send a structurally identical list on every page (and | ||
| // after an exact local collapse patch): keep the previous reference so | ||
| // the grid does not rebuild its O(total rows) linear layout for nothing | ||
| setGroupPoints((prev) => (isEqual(prev, freshGroupPoints) ? prev : freshGroupPoints)); |
There was a problem hiding this comment.
After a same-view filter, sort, or row-hiding search change, the layout effect clears groupRowCounts; if the authoritative delivery has structurally equal group points—which is common when sorting within groups or when a filter preserves each group's count—this equality branch retains the old state reference, so the effect that collects counts never runs again. Collapsing a group then omits its now-missing count, and re-expanding it takes the unknown-size path that drops every loaded row behind it until another delivery arrives. Refresh the count cache from every authoritative delivery even when the React layout state can retain its reference.
Useful? React with 👍 / 👎.
tea-artist
changed the title
e600e6e to
49b46d0
Compare
tea-artist
changed the title
49b46d0 to
d6ab6e0
Compare
tea-artist
changed the title
d6ab6e0 to
bee7cb1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bee7cb196b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }) | ||
| } | ||
| onCreate={async (role) => { | ||
| if (await confirmSeat({ role, count: 1, action: 'link' })) { |
There was a problem hiding this comment.
On an EE instance already at its seat limit, this count: 1 makes useSeatConfirm reject creating a billable-role link even though link creation consumes no seat: InvitationService.generateInvitationLink only creates the invitation, while a collaborator is added later by acceptInvitationLink. The eventual accepter may also already be billable, so capacity cannot be determined at link-creation time; retain the Cloud warning but do not apply the EE seat-delta gate here. The space-link creation handler has the same problem.
Useful? React with 👍 / 👎.
| const isInIframe = useIsInIframe(); | ||
| const [dismissed, setDismissed] = useState(false); | ||
|
|
||
| if (!isHydrated || !isMobile || isInIframe || dismissed) { |
There was a problem hiding this comment.
When a ?embed=... share URL is opened outside an actual iframe on a viewport at or below 640px, isInIframe is false and this condition renders the fixed edit/save bar. The share view components consistently treat the query parameter itself as embed mode and suppress their headers, so a direct embed preview or new-tab embed now gains an operation overlay despite requesting chrome-free embed output. Check router.query.embed here in addition to the window iframe state.
Useful? React with 👍 / 👎.
| const handleUpdateRole = useCallback( | ||
| (role: IRole, item: CollaboratorItem) => { | ||
| async (role: IRole, item: CollaboratorItem) => { | ||
| const addedSeats = item.type === PrincipalType.User && !item.billable ? 1 : 0; |
There was a problem hiding this comment.
For the base and space collaborator-list responses implemented in this tree, CollaboratorService.getListByBase and getListBySpace never populate the optional billable property, so every user reaches this expression with item.billable === undefined and is treated as a new seat. Consequently, changing an already billable user from Creator to Editor shows a false Cloud cost-increase confirmation and is blocked entirely on an EE instance at its seat limit. Populate the billing status or obtain the actual server-side seat delta before gating the role update; the invitation-dialog role handlers repeat this assumption.
Useful? React with 👍 / 👎.
tea-artist
changed the title
bee7cb1 to
f2f75f1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2f75f1457
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (connection?.saveData || connection?.effectiveType === '2g') { | ||
| return; |
There was a problem hiding this comment.
When the Network Information API reports effectiveType === 'slow-2g' and data saver is disabled, this condition still schedules the heavy Table chunk download. Since slow-2g is an explicit effective-connection type and is slower than 2g, the users most affected by this speculative download bypass the protection described by the hook; exclude both 2g and slow-2g before warming the bundles.
Useful? React with 👍 / 👎.
tea-artist
changed the title
f2f75f1 to
a937e10
Compare
tea-artist
changed the title
a937e10 to
0985fee
Compare
tea-artist
changed the title
0985fee to
7a06430
Compare
tea-artist
changed the title
7a06430 to
0fe137f
Compare
tea-artist
changed the title
0fe137f to
becce22
Compare
tea-artist
changed the title
becce22 to
d75e6a3
Compare
tea-artist
changed the title
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
20 similar comments
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
💡 Codex Review
Preserve self-table field-reference semantics in the set-based path
When a conditional lookup targets its own table and compares two fields (for example NameMirror is Name), this newly added fast path reconstructs the filter and sends it through buildFilterConditionWhere while treating foreignTable and the current table as separate sides. Because they are the same table, the field-reference mapping resolves the filter/reference sides inconsistently and generates SQL against the wrong alias/column (the existing self-table regression cases report references such as h.NameMirror/h.Name2 and a 500 during field creation). Detect self-table references and use the correlated path, or build the predicate with an explicit host/foreign field mapping.
Drop legacy unique indexes when disabling a field's uniqueness
When a conversion changes unique from true to false, this only drops the conventionally named ${table}_${column}_unique constraint/index. Existing deployments can still have the legacy field-ID-named unique index created by the V1 field service, which remains active after the metadata says uniqueness is disabled and continues rejecting duplicate writes. Discover and remove the actual unique constraint/index for the column (or handle both naming schemes) rather than relying on one generated name.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const buildMapTextSql = (metadata: LinkMappingMetadata): string | null => { | ||
| if (!metadata.lookupColumnName || !metadata.foreignTable) { | ||
| return null; |
There was a problem hiding this comment.
When a linked table is permanently deleted, getPreviewLinkMappingMetadata cannot provide foreignTable, so this guard returns null; the conversion still drops the JSON link column and creates the text column, but no mapping statement copies the stored link object's title. Every inbound link value is therefore lost during the link-to-text downgrade instead of remaining readable after the target table is gone. The migration should extract titles from the preserved link JSON when the foreign table is unavailable, or retain the old value until a mapping can be performed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2ba2484fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const visibleRowSearch = resolveVisibleRowSearch( | ||
| RecordSearch.fromOptionalTuple(query.search), | ||
| searchVisibleFieldIds | ||
| requestedSearch, | ||
| projectedSearchVisibleFieldIds | ||
| ); |
There was a problem hiding this comment.
When a caller supplies a highlight-only search such as search: ['text'] (the default hideNotMatchRow is false), resolveVisibleRowSearch returns undefined here, so the repository receives no search descriptor even though includeSearchFieldMatches is enabled by the V2 record service. buildRecordSearchFieldMatches therefore produces no per-field matches and extra.searchHitIndex is empty, regressing grid highlighting and socket doc-id highlights; pass the search descriptor separately for match extraction while keeping it out of the row predicate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4806921313
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const count = filledCellCount(rows[index]); | ||
| if (count > bestCount) { | ||
| bestCount = count; | ||
| bestIndex = index; |
There was a problem hiding this comment.
When a normal worksheet's header is less populated than any of its first 30 data rows—for example, two named columns plus a blank header whose data cells are populated—this selects the first wider data row as the header. createRowsIterable then starts at that row, silently discarding every preceding record and using cell values as field names. Detect a leading banner without treating the widest scanned row as the header for ordinary sheets.
Useful? React with 👍 / 👎.
| if (!Number.isFinite(expiresMs)) { | ||
| return Math.floor(Date.now() / 1000); | ||
| } | ||
| return Math.floor(expiresMs / 1000) - this.ttl; |
There was a problem hiding this comment.
When a concurrent session-map update has orphaned a session—the exact case this recovery path handles—clearByUserId cannot enumerate and delete that session, so this timestamp comparison is the only revocation fence. However, stored cookies expire one year after issue (session-handle.service.ts), while this subtracts the roughly seven-day store TTL, producing a renewal timestamp about 358 days in the future. The old session therefore compares newer than the clear marker and is restored after password reset, password change, or email change; persist a real issue/renewal timestamp instead of deriving it with the store TTL.
Useful? React with 👍 / 👎.
| for await (const chunk of stream) { | ||
| if (deadline !== undefined && Date.now() > deadline) { | ||
| stream.destroy(); | ||
| throw new ColdReadDeadlineError(`download of ${key} exceeded the cold read budget`); |
There was a problem hiding this comment.
When an object-store GET returns a readable stream and then stalls before emitting the next chunk, the for await remains blocked and never reaches this deadline check. S3 and Aliyun clients are constructed without socket/request timeouts, so cold-backed history and removal reads can remain occupied indefinitely despite their configured read budget. Race the read against a timer or abort/destroy the stream at the deadline rather than checking only after chunks arrive.
Useful? React with 👍 / 👎.
| if (isEncryptedAiConfigValue(plaintext)) { | ||
| return plaintext; |
There was a problem hiding this comment.
When an administrator supplies an API key beginning with teable_enc_v1:—the provider schema accepts arbitrary strings—this branch assumes the plaintext is already encrypted and stores it verbatim. On reads, GCM decryption fails and decryptAiConfigSecrets returns the same value, so a custom provider can continue receiving the key while its supposedly protected credential remains plaintext in the database. Use an authenticated envelope or separate storage tag rather than treating a user-controlled prefix alone as proof of encryption.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d229fe635a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const userIdExpression = viewCollaboratorFieldIsMultiple(field) | ||
| ? sql<string>`jsonb_array_elements(COALESCE(${column}::jsonb, '[]'::jsonb))->>'id'` | ||
| : sql<string>`${column}::jsonb->>'id'`; |
There was a problem hiding this comment.
When a multi-value collaborator field contains a legacy or compatibility-form scalar object instead of a JSON array, this expression calls jsonb_array_elements directly on that object and PostgreSQL raises cannot extract elements from an object. The query then returns an infrastructure error and the view-collaborator endpoint fails, even though the rest of the record query code explicitly accepts object-or-array shapes; wrap objects in a one-element array (and treat other JSON types as empty) before expansion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Normalize single-link values before returning created records
When this V2 create path is used with typecast: true and a many-one link field, the link title resolver produces its result in array form, but the V2 response mapper returns that value without applying the single-relationship normalization used by the legacy path. As a result, clients receive [{"id":...,"title":...}] for a single link instead of the documented { "id": ..., "title": ... } shape, breaking consumers that deserialize the field according to its relationship. Normalize each created record according to the target field's multiplicity before returning it.
teable/apps/nestjs-backend/src/features/record/open-api/record-open-api-v2.service.ts
Lines 3830 to 3844 in 81624df
Allow deletion when an errored link references a missing table
When a table contains a link field whose foreign table was deleted, the V2 delete path now loads every selected record before issuing the delete. Rehydrating that table rejects the invalid foreignTableId, so deleting records fails before any deletion occurs, whereas the legacy path tolerates the errored link field and still removes the records. Make the snapshot read tolerant of invalid link metadata or use a delete query that does not require rehydrating the full table aggregate.
teable/apps/nestjs-backend/src/features/field/open-api/field-open-api-v2.service.ts
Lines 1525 to 1535 in 81624df
Support self-referencing field-reference lookups
When creating a conditional lookup whose foreign table is the same table and whose filter compares one field to another field, this V2 create path sends the self-table configuration into the set-based backfill planner. That planner treats the filter field as belonging to the foreign side, generates references to host columns that do not exist in the generated query, and the field creation fails with a database error instead of creating the lookup. Handle self-table field-reference operands with the correct host/foreign aliases before executing the backfill.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (oldField.unique().toBoolean() && !newField.unique().toBoolean()) { | ||
| const constraintName = `${tableName}_${dbFieldName}_unique`; | ||
| const quotedIndexName = schema ? `"${schema}"."${constraintName}"` : `"${constraintName}"`; | ||
| constraintCleanupStatements.push({ | ||
| scope: 'data', | ||
| compile: () => | ||
| sql`ALTER TABLE ${sql.raw(fullTableName)} DROP CONSTRAINT IF EXISTS ${sql.ref(constraintName)}`.compile( | ||
| visitor.params.db | ||
| ), | ||
| }); | ||
| constraintCleanupStatements.push({ | ||
| scope: 'data', | ||
| compile: () => | ||
| sql`DROP INDEX IF EXISTS ${sql.raw(quotedIndexName)}`.compile(visitor.params.db), |
There was a problem hiding this comment.
When a field has a standalone unique index left by the V1 schema (whose name is based on the field ID) and V2 changes unique to false, this cleanup only derives the new <table>_<column>_unique name. The old index remains active, so writes that reuse a formerly unique value still fail even though the field metadata now says uniqueness is disabled. Discover and drop all unique indexes attached to the column, not just the V2-generated name.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06dba5cce0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return ok({ totalImported: 0, events: [] }); | ||
| } | ||
|
|
||
| const totalRecordCount = rows?.length ?? 0; |
There was a problem hiding this comment.
When a CSV import is read through rowsAsync (for example, a URL-backed large import), rows is intentionally undefined, so this sets totalRecordCount to 0 even though many records may be inserted. The emitted RecordsBatchCreated events carry that value into RecordsBatchCreatedRealtimeProjection, whose large-batch policy skips realtime fanout only when the total is at least 1,000; streamed imports therefore fan out every batch to every subscribed client instead of using the large-import refresh path, causing avoidable realtime load and potentially overwhelming clients. Track the count while consuming the stream or otherwise determine it before setting orchestration metadata.
Useful? React with 👍 / 👎.
| // 按导入列定义匹配，保留 sourceColumnIndex 指向的源列。 | ||
| for (let i = 0; i < columns.length && i < fields.length; i++) { | ||
| const header = headers[columns[i].sourceColumnIndex]; | ||
| const field = fields[i]; | ||
| map.set(header, field.id().toString()); |
There was a problem hiding this comment.
When the caller supplies columns entries that reference the same sourceColumnIndex, the service creates multiple destination fields but this map stores only the last field ID for that source header. Every imported value is therefore written to the last duplicate field while the earlier field remains empty, with no validation error explaining the malformed mapping. Either reject duplicate source indexes or preserve a one-to-many mapping and copy the value to each selected field.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b0d307658
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| const resolvedColumns: ResolvedImportColumn[] = []; | ||
| for (const column of columns) { | ||
| if (column.sourceColumnIndex < 0 || column.sourceColumnIndex >= headers.length) { |
There was a problem hiding this comment.
When an import request supplies a fractional sourceColumnIndex (the public importColumnSchema currently accepts any number), this range check passes, but headers[column.sourceColumnIndex] and the later field map lookup use that fractional property as an array key. The resulting field is created with no matching source header, so every imported row leaves it empty instead of returning a validation error. Require an integer index before resolving the column.
Useful? React with 👍 / 👎.
Synced from teableio/teable-ee@f91f743 Co-authored-by: Aries X <caoxing9@gmail.com> Co-authored-by: Bieber <artist@teable.io> Co-authored-by: Boris <boris2code@outlook.com> Co-authored-by: Gary Guangyu Li <gary@teable.ai> Co-authored-by: Jocky Zhou <jocky@teable.ai> Co-authored-by: Jun Lu <hammond@teable.io> Co-authored-by: Pengap <penganpingprivte@gmail.com> Co-authored-by: SkyHuang <sky.huang.fe@gmail.com> Co-authored-by: Uno <uno@teable.ai> Co-authored-by: nichenqin <nichenqin@hotmail.com> Co-authored-by: teable-mini[bot] <310066019+teable-mini[bot]@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abf2bb2837
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (source.stream) { | ||
| return this.parseData(await collectAsyncText(source.stream), options); |
There was a problem hiding this comment.
When source.stream is used for a large CSV, this first concatenates the entire stream into one string and then Papa.parse materializes all rows, so memory usage grows with the whole file (and temporarily includes both the string and parsed row structures). This defeats the streaming import contract and can OOM the backend on files that were intended to be processed incrementally; expose an async row iterator and parse chunks as they arrive instead.
Useful? React with 👍 / 👎.
| "./table-query-ops": { | ||
| "@teable/source": "./src/tableQueryOps.ts", | ||
| "types": "./src/tableQueryOps.ts", | ||
| "import": "./dist/tableQueryOps.js", | ||
| "require": "./dist/tableQueryOps.cjs" |
There was a problem hiding this comment.
The newly added ./table-query-ops export points its ESM import at dist/tableQueryOps.js, so a clean workspace consumer such as Vitest/Vite fails unless this package has already been built. This new subpath should follow the repository's source-visibility contract and point import at src/tableQueryOps.ts (while retaining the compiled CommonJS entry). agents.mdL44-L58
Useful? React with 👍 / 👎.
2 participants
🔄 Automated sync from EE repository.
278 commit(s) synced since last sync.
Authors
Included commits
Latest source commit: teableio/teable-ee@f91f743
This PR was automatically created by the sync workflow.